ENG-9350: App.modify_state can directly modify SharedState tokens#6336
ENG-9350: App.modify_state can directly modify SharedState tokens#6336
Conversation
When a SharedState is modified directly by its shared token (e.g. from an API route webhook), propagate dirty vars to all clients linked to that shared state — matching the behavior that already exists when modifying shared state through a private client token. - Add _collect_shared_token_updates on SharedStateBaseInternal to detect the shared-token case inside _modify_linked_states and propagate to linked clients via the existing _do_update_other_tokens mechanism - Add App.set_contexts / App._set_contexts_internal to centralize pushing RegistrationContext and EventContext into the current contextvars scope, replacing the old _registration_context_middleware - App.modify_state now calls set_contexts so out-of-band callers (API routes, webhooks) get the necessary contexts automatically - Integration test: API endpoint modifies two SharedState subclasses by shared token, asserts both propagate to two linked browser tabs, and verifies normal event handlers still work afterward - Unit tests for set_contexts covering all combinations of pre-existing / absent contexts, no-event-processor, and reset-on-exit behavior Closes #6335
Greptile SummaryThis PR enables The implementation adds Confidence Score: 5/5Safe to merge; the only finding is a minor consistency nit with no impact on current behaviour. The core logic — detecting the shared-token path via empty _reflex_internal_links, delegating to _collect_shared_token_updates, and injecting contexts via set_contexts — is correct. The truthiness-vs-is-not-None difference in the new helper is a style inconsistency only; _previous_dirty_vars is always non-empty when vars are genuinely dirty. Integration and unit tests cover multi-SharedState propagation, context setup/teardown for all four pre-existing-context combinations, and verified post-API normal event handling. reflex/istate/shared.py — the _previous_dirty_vars truthiness check (P2 style nit). Important Files Changed
Sequence DiagramsequenceDiagram
participant API as API Route / Webhook
participant App as App.modify_state
participant SC as set_contexts()
participant SM as StateManager
participant SBI as SharedStateBaseInternal
participant CS as _collect_shared_token_updates
participant DU as _do_update_other_tokens
participant C1 as Client 1 (private token)
participant C2 as Client 2 (private token)
API->>App: modify_state(BaseStateToken(shared_token, SharedState))
App->>SC: set_contexts() → push RegistrationContext + EventContext
App->>SM: modify_state_with_links(shared_token)
SM->>SBI: _modify_linked_states()
Note over SBI: _reflex_internal_links is empty (shared token case), _held_locks remains {}
SBI->>SBI: yield (caller modifies ss.counter, notes.note)
Note over SBI: _held_locks_linked_states() returns [] → normal loop is no-op
SBI->>CS: _collect_shared_token_updates(affected_tokens, current_dirty_vars)
CS->>CS: iterate substates, add _linked_from clients to affected_tokens, populate current_dirty_vars
CS-->>SBI: affected_tokens = {client1, client2}, current_dirty_vars = {...}
SBI->>DU: _do_update_other_tokens(affected_tokens, current_dirty_vars)
DU->>C1: emit_update (delta for counter + note)
DU->>C2: emit_update (delta for counter + note)
App->>SC: reset contexts on exit
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if substate._previous_dirty_vars: | ||
| current_dirty_vars[substate.get_full_name()] = set( | ||
| substate._previous_dirty_vars | ||
| ) | ||
| if substate._get_was_touched() or substate._previous_dirty_vars: | ||
| affected_tokens.update(substate._linked_from) |
There was a problem hiding this comment.
is not None pattern
The existing collection loop (lines 376–383) uses is not None to decide whether to populate current_dirty_vars and affected_tokens, treating an empty set() the same as a populated one. This new method uses a truthiness check, so an empty _previous_dirty_vars = set() (the field's initialiser value) skips the current_dirty_vars entry even when _get_was_touched() is True.
In practice _previous_dirty_vars will be non-empty whenever vars were actually modified, so this is unlikely to surface as a bug; but aligning with the is not None guard used in the sibling loop keeps the two paths consistent and easier to reason about.
| if substate._previous_dirty_vars: | |
| current_dirty_vars[substate.get_full_name()] = set( | |
| substate._previous_dirty_vars | |
| ) | |
| if substate._get_was_touched() or substate._previous_dirty_vars: | |
| affected_tokens.update(substate._linked_from) | |
| if substate._previous_dirty_vars is not None: | |
| current_dirty_vars[substate.get_full_name()] = set( | |
| substate._previous_dirty_vars | |
| ) | |
| if substate._get_was_touched() or substate._previous_dirty_vars is not None: | |
| affected_tokens.update(substate._linked_from) |
set an initial value for the shared state to provide affirmative proof that the state was linked _before_ the API call was made. if only one state was linked when the HTTP request was made, then only one state would be updated by such request resulting in the observed behavior.
but actually set the value on the newly linked state, not the old private state. derpy easy to make mistake with this _link_to mechanism =/
if the linked state is not fetched in the initial request, then make sure we check if it should be patched in when performing a `get_state` call
1 participant
When a SharedState is modified directly by its shared token (e.g. from an API route webhook), propagate dirty vars to all clients linked to that shared state — matching the behavior that already exists when modifying shared state through a private client token.
Closes #6335